Fix BTD/Scrape Flush Count with Filters #4657
Conversation
ax3l
added
bug
| @@ -399,8 +399,6 @@ private: | |||
| lab-frame data. */ | |||
| void InitializeParticleFunctors () override; | |||
|
|
|||
| /** Update total number of particles flushed for all species for ith snapshot */ | |||
| void UpdateTotalParticlesFlushed(int i_buffer); | |||
There was a problem hiding this comment.
Removed and delegated to the I/O backend, which currently does the filtering and thus knows what number of particles is truly written.
There was a problem hiding this comment.
is this also done for plotfile?
There was a problem hiding this comment.
ah I see - its not needed for plotfile - because we just merge headers and interleave data . this was needed mainly for openpmd.
all good!
|
cc @roelof-groenewald just FYI for scraping with filters |
ax3l
force-pushed
the
fix-btd-scrape-offset-cnt
branch
4 times, most recently
from
081e7c1 to
140ba0b
Compare
This comment was marked as resolved.
This comment was marked as resolved.
ax3l
force-pushed
the
fix-btd-scrape-offset-cnt
branch
2 times, most recently
from
43883bb to
a44f743
Compare
ax3l
force-pushed
the
fix-btd-scrape-offset-cnt
branch
from
a44f743 to
39d510f
Compare
|
Just checked, the input file in issue 4604 works! |
|
I noticed orders of inputs are shuffled in several functions. Why is that? |
I pass the offsets as references now, thus I cannot use default parameters anymore. |
Yes, I saw that. But orders of inputs are different. For example totalParticlesFlushedAlready was placed a few lines up in FlushFormat.H. Just curious. |
Move the counting of already flushed particles for writers that call the I/O backends multiple time per data set, e.g., BTD and boundary scraping, into the I/O backend. Currently, filtering is done as the first step in I/O backends and thus the previous count outside of the I/O backends was over-counting particles that might still get filtered out. Offset should be a `long`: Overflow risk is very high for pure `int`. Also, counter is `unsigned`, so `unsigned long` for now.
Less state we can forget in checkpoint-restart and that we have to transfer across API boundaries.
ax3l
force-pushed
the
fix-btd-scrape-offset-cnt
branch
from
39d510f to
2483724
Compare
|
Simplified it further to get rid of state that we will forget during checkpoint-restart :) Too much back and forth over the frontend-backend APIs. |
| WARPX_ALWAYS_ASSERT_WITH_MESSAGE( | ||
| std::rename(recent_ParticleHdrFilename.c_str(), snapshot_ParticleHdrFilename.c_str()) == 0, | ||
| std::string("Renaming ").append(recent_ParticleHdrFilename).append(" to ").append(snapshot_ParticleHdrFilename).append(" has failed")); | ||
| if (!amrex::FileExists(snapshot_ParticleHdrFilename)) { |
There was a problem hiding this comment.
@RevathiJambunathan @atmyers highlighting this: I want to remove m_totalParticles_flushed_already because it is brittle state.
Let me know if you would add another check for this line.
There was a problem hiding this comment.
I think this is good!
|
Looks good to me! @ax3l :) Thanks for this PR |
Move the counting of already flushed particles for writers that call the I/O backends multiple time per data set, e.g., BTD and boundary scraping, into the I/O backend.
Currently (until #4420), filtering is done as the first step in I/O backends and thus the previous count outside of the I/O backends was over-counting particles that might still get filtered out.
Regression to #4570.
Fix #4604.